Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting
| fn get(&self, key: &Key) -> Option<Value> { | ||
| Python::with_gil(|py| { | ||
| let bound = self.mapping.bind(py); | ||
| let mapping = bound.downcast::<PyMapping>().ok()?; | ||
| let py_key = Self::key_to_python(py, key); | ||
| let value = mapping.get_item(py_key).ok()?; | ||
| RustyPyType(&value).try_into_value().ok() | ||
| }) |
There was a problem hiding this comment.
When a value in a lazy Python mapping fails to convert into a CEL Value, PyMappingValue::get returns None and the evaluator treats the access as a missing key (null). This silently discards the underlying Python exception, whereas the previous eager conversion of PyDict raised a ConversionError immediately. A dict subclass that yields an unsupported object for data["foo"] will now evaluate to null instead of raising, making bad data hard to detect. It would be better to surface the conversion failure (e.g., by raising or logging) rather than swallowing it as an absent key.
Useful? React with 👍 / 👎.
|
superseded by the mapping-protocol fix |
Both `uses:` refs were `@main`, where neither template exists: they arrive with ci-templates#27, and hardbyte#24 — the PR this file's sequencing note points at — is closed with its branch deleted. Armed as written, the run would fail with `workflow was not found` rather than publishing anything. Pinned to 53a515b0, the tip of that PR, which also carries the post-cutover CodeArtifact role defaults. The inputs this caller passes are unchanged there: codeartifact-build-rust still takes `interpreters` and emits `artifact_pattern`, and codeartifact-publish still takes `package_name`, `artifact_pattern` and `environment`. A pinned ref also closes the mutable-ref finding: this caller grants `id-token: write`, so a later change to the called workflow could act on it. actionlint and zizmor both clean.
1 participant
Summary
MapValuetrait so maps can be backed by dynamic sourcesPyMappingValueand surface them to CEL without eagerly copying entries__getitem__calls are evaluated correctlyTesting
cargo test(fails: missing libpython3.11.so.1.0 in test harness)pytest(fails: environment lacks installedcelpackage and mktestdocs dependency)https://chatgpt.com/codex/tasks/task_b_68ede34e85148329ad0078eeea94b756